Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add FwHeadless (renamed from CrdtMerge) deployment for k8s and GHA #1171

Merged
merged 16 commits into from
Oct 31, 2024

Conversation

rmunn
Copy link
Contributor

@rmunn rmunn commented Oct 28, 2024

Fix #1163.

This adds a new k8s deployment for FwHeadless (formerly called CrdtMerge) in its own pod, and adds GHA workflow steps to build and deploy it just like our other pods.

New secret named fw-headless should be created in prod environment, containing the username and password of a user that will be allowed to Send/Receive any project. For local-dev, develop, and staging this should just be "admin" and the password from SEED_USER_PASSWORD, but for production we'll want a dedicated CrdtMerge user, so that if CrdtMerge credentials leak we can revoke that user's access and then set up a new one. (Or we could reuse the account that lfmerge is using, that would also work).

@rmunn rmunn self-assigned this Oct 28, 2024
Copy link

github-actions bot commented Oct 28, 2024

UI unit Tests

12 tests  ±0   12 ✅ ±0   0s ⏱️ ±0s
 4 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit cad8585. ± Comparison against base commit a84c509.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Oct 28, 2024

C# Unit Tests

75 tests  ±0   75 ✅ ±0   5s ⏱️ ±0s
13 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit cad8585. ± Comparison against base commit a84c509.

♻️ This comment has been updated with latest results.

@hahn-kev
Copy link
Collaborator

If you end up not making a copy of develop-api.yaml and you want to filter based on paths changed you can use this action
https://github.com/marketplace/actions/paths-changes-filter

@rmunn rmunn marked this pull request as ready for review October 29, 2024 05:06
@rmunn rmunn requested a review from hahn-kev October 29, 2024 05:06
Copy link
Contributor Author

@rmunn rmunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review completed. You might wonder if the SendReceiveConfig__Foo variables in k8s should be CrdtMergeConfig, but in fact the CrdtMerge code actually uses the name "SendReceiveConfig" when it binds to the app settings. I do think I should rename that to CrdtMergeConfig, but it's probably better to do that in a later PR after this one is merged, so that I can rename that app settings name everywhere all at once.

Copy link
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I left 2 comments to look at.

One issue that's not covered here, I'm not really sure where to cover it, is that by default this won't work on staging/develop/production because there's no auth for the S&R user. However we have the default password of our generated users, so for non production we can actually use username: 'admin' and the password from the secret 'db' key 'SEED_USER_PASSWORD'

deployment/base/crdtmerge-deployment.yaml Outdated Show resolved Hide resolved
deployment/base/crdtmerge-deployment.yaml Outdated Show resolved Hide resolved
@rmunn
Copy link
Contributor Author

rmunn commented Oct 30, 2024

One issue that's not covered here, I'm not really sure where to cover it, is that by default this won't work on staging/develop/production because there's no auth for the S&R user. However we have the default password of our generated users, so for non production we can actually use username: 'admin' and the password from the secret 'db' key 'SEED_USER_PASSWORD'

I thought about using that and decided that perhaps a new secret was better, but on re-examination this is probably better because we won't need to make any changes to develop and staging. So I implemented this in commit 6028ea1.

rmunn added 7 commits October 30, 2024 13:39
Instead of defining a new secret that will have to be added to develop
and staging environments, we can reuse SEED_USER_PASSWORD and have
things "just work". Then only prod will need a separate user account
created and its credentials stored in a new secret.
@rmunn rmunn force-pushed the feat/deploy-crdtmerge branch from cd2b7e7 to 6028ea1 Compare October 30, 2024 06:40
@rmunn rmunn requested a review from hahn-kev October 30, 2024 06:42
@rmunn
Copy link
Contributor Author

rmunn commented Oct 30, 2024

@hahn-kev - Addressed your review comments, should be ready for re-review now.

Also renamed SendReceiveConfig section of appsettings.json to
FwHeadlessConfig so we get all the renames done at once.
@rmunn rmunn changed the title Add CrdtMerge deployment for k8s and GHA Add FwHeadless (renamed from CrdtMerge) deployment for k8s and GHA Oct 30, 2024
@rmunn
Copy link
Contributor Author

rmunn commented Oct 30, 2024

Commit 140de48 has the big CrdtMerge -> FwHeadless rename.

Copy link
Contributor

@myieye myieye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me take a moment to express how thankful I am that I didn't have to do this!!
This probably would have taken me 2 weeks. This stuff just makes my eyes bleed. I don't understand how you guys produce all this crazy yaml.

deployment/base/pvc.yaml Show resolved Hide resolved
rmunn added 7 commits October 31, 2024 08:41
Will request:

- 10Gi on GHA
- 20Gi on local-dev
- 20Gi on develop
- 40Gi on staging
- 80Gi on prod
This reverts commit acf0a27.

We'll do this the simpler way.
Now we allocate 10 GiB everywhere, except 20 on staging and 150 GiB on
production (similar size to hg-repos volume since we could potentially
end up with copies of everything from hg-repos).
Chorus uses the XDG_DATA_HOME environment variable to save its settings.
It doesn't exist in the base .NET SDK image, so we need to set it. We
set it in the Dockerfile rather than in the k8s deployment because it
should be the same everywhere.
Should make for cleaner shutdown and restart on deployment
Copy link
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me

@rmunn rmunn merged commit c7c515d into develop Oct 31, 2024
17 checks passed
@rmunn rmunn deleted the feat/deploy-crdtmerge branch October 31, 2024 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FwHeadless container and k8s deployment
3 participants